Support pull diagnostics and use them for testing #2166
Conversation
|
Bencher Report| Branch | more-reliable-nls-testing |
| Testbed | ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | microseconds (µs) |
|---|---|---|
| fibonacci 10 | 📈 view plot 🚷 view threshold | 386.41 |
| foldl arrays 50 | 📈 view plot 🚷 view threshold | 1,571.70 |
| foldl arrays 500 | 📈 view plot 🚷 view threshold | 5,818.90 |
| foldr strings 50 | 📈 view plot 🚷 view threshold | 5,834.10 |
| foldr strings 500 | 📈 view plot 🚷 view threshold | 50,933.00 |
| generate normal 250 | 📈 view plot 🚷 view threshold | 46,578.00 |
| generate normal 50 | 📈 view plot 🚷 view threshold | 1,876.80 |
| generate normal unchecked 1000 | 📈 view plot 🚷 view threshold | 2,833.80 |
| generate normal unchecked 200 | 📈 view plot 🚷 view threshold | 653.43 |
| pidigits 100 | 📈 view plot 🚷 view threshold | 2,756.20 |
| pipe normal 20 | 📈 view plot 🚷 view threshold | 1,277.90 |
| pipe normal 200 | 📈 view plot 🚷 view threshold | 8,479.20 |
| product 30 | 📈 view plot 🚷 view threshold | 747.47 |
| scalar 10 | 📈 view plot 🚷 view threshold | 1,312.10 |
| sum 30 | 📈 view plot 🚷 view threshold | 746.70 |
There was a problem hiding this comment.
If I understand correctly, this adds a new capability to NLS - send current document diagnostics - and use that in the test harness instead of looping over the answers.
The format is slightly less nice for snapshot test, but 1. it's not very important and 2. it's limited to eval diagnostics, if I understand correctly. Let's try this PR on top of one of the failing PR and see if it seems to solve the problem.
There was a problem hiding this comment.
Let's try this way. It seems to work in #2173, although the failures aren't always deterministic. This still adds a new capability to NLS, and the price is minimal (beside added LoC, the snapshot for eval diagnostics are slightly less nice, but it's not user-facing)
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Push diagnostics (where the LSP decides when to publish, and sends notifications) are hard to test reliably, because the harness doesn't know when and how many diagnostics to expect.
This PR adds support for "pull" diagnostics, where there's a diagnostic request and the server responds to it. This simplifies the testing, and hopefully stops it from timing out at random.
This will probably conflict with the typechecker changes. I'm happy to wait and rebase.